[java][BiDi] implement `browsingContext.downloadEnd` event #16347

Delta456 · 2025-09-24T11:54:38Z

User description

🔗 Related Issues

💥 What does this PR do?

Implements browsingContext.downloadEnd event according to spec https://w3c.github.io/webdriver-bidi/#event-browsingContext-downloadEnd for Java binding.

ONLY supported for Chrome atm.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement

Description

Implement browsingContext.downloadEnd event for Java BiDi binding
Add support for download completion and cancellation states
Create new classes for handling download end scenarios
Update test to verify download end event functionality

Diagram Walkthrough

flowchart LR
  A["DownloadEnded"] --> B["DownloadCompleted"]
  A --> C["DownloadCanceled"]
  D["BrowsingContextInspector"] --> E["onDownloadEnd listener"]
  E --> A

File Walkthrough

Relevant files

Enhancement

DownloadCanceled.java `Add DownloadCanceled class for canceled downloads` java/src/org/openqa/selenium/bidi/browsingcontext/DownloadCanceled.java New class extending NavigationInfo for canceled downloads Implements JSON deserialization with status validation Provides getter for download status	+79/-0
DownloadCompleted.java `Add DownloadCompleted class for successful downloads` java/src/org/openqa/selenium/bidi/browsingcontext/DownloadCompleted.java New class extending NavigationInfo for completed downloads Includes filepath property for download location Implements JSON parsing with status validation	+95/-0
DownloadEnded.java `Add DownloadEnded wrapper for download end events` java/src/org/openqa/selenium/bidi/browsingcontext/DownloadEnded.java Main wrapper class for download end events Factory method creates appropriate subclass based on status Provides helper methods to check download state	+93/-0
BrowsingContextInspector.java `Add download end event support to inspector` java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java Add downloadWillEndMapper for JSON deserialization Create downloadWillEndEvent for browsingContext.downloadEnd Add onDownloadEnd method for event subscription Update close method to clear new event listener	+21/-6

Tests

BrowsingContextInspectorTest.java `Update test for download end event` java/test/org/openqa/selenium/bidi/browsingcontext/BrowsingContextInspectorTest.java Update test from downloadWillBegin to downloadEnd event Verify download completion status and context ID Test download end event functionality	+8/-8

qodo-merge-pro · 2025-09-24T11:55:29Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 5678 - Partially compliant Compliant requirements: Non-compliant requirements: Investigate/resolve ChromeDriver connection failure issue. Requires further human verification: Reproduce connection failure across multiple ChromeDriver instantiations on Ubuntu 16.04 environment. Validate compatibility matrix (Chrome/ChromeDriver/Selenium versions) and networking/environmental causes. 1234 - Partially compliant Compliant requirements: Non-compliant requirements: Address click() not triggering javascript in href on Firefox. Requires further human verification: Re-run the provided test case on affected Firefox/WebDriver versions to confirm behavior.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Null Handling If the incoming JSON lacks a 'status' field, an IllegalArgumentException is thrown. Confirm that the BiDi payload always includes 'status' per spec; otherwise consider a clearer error message or default. // Create the appropriate object based on status if ("canceled".equals(status)) { DownloadCanceled canceled = new DownloadCanceled(browsingContextId, navigationId, timestamp, url, status); return new DownloadEnded(canceled); } else if ("complete".equals(status)) { DownloadCompleted completed = new DownloadCompleted(browsingContextId, navigationId, timestamp, url, status, filepath); return new DownloadEnded(completed); } else { throw new IllegalArgumentException( "status must be either 'canceled' or 'complete', but got: " + status); } Field Optionality 'filepath' may be absent depending on browser implementation; ensure downstream consumers handle null gracefully and consider documenting nullability. case "filepath": filepath = input.read(String.class); break; default: input.skipValue(); break; } } input.endObject(); return new DownloadCompleted(browsingContextId, navigationId, timestamp, url, status, filepath); } public String getStatus() { return status; } public String getFilepath() { return filepath; } Listener Lifecycle New downloadEnd listener is added and cleared, but ensure symmetry when multiple onDownloadEnd registrations occur and verify no leaks with navigationEventSet interactions. private final Event<DownloadEnded> downloadWillEndEvent = new Event<>("browsingContext.downloadEnd", downloadWillEndMapper); private final Event<UserPromptOpened> userPromptOpened = new Event<>( "browsingContext.userPromptOpened", params -> { try (StringReader reader = new StringReader(JSON.toJson(params)); JsonInput input = JSON.newInput(reader)) { return input.read(UserPromptOpened.class); } }); private final Event<HistoryUpdated> historyUpdated = new Event<>( "browsingContext.historyUpdated", params -> { try (StringReader reader = new StringReader(JSON.toJson(params)); JsonInput input = JSON.newInput(reader)) { return input.read(HistoryUpdated.class); } }); public BrowsingContextInspector(WebDriver driver) { this(new HashSet<>(), driver); } public BrowsingContextInspector(String browsingContextId, WebDriver driver) { this(Collections.singleton(Require.nonNull("Browsing context id", browsingContextId)), driver); } public BrowsingContextInspector(Set<String> browsingContextIds, WebDriver driver) { Require.nonNull("WebDriver", driver); Require.nonNull("Browsing context id list", browsingContextIds); if (!(driver instanceof HasBiDi)) { throw new IllegalArgumentException("WebDriver instance must support BiDi protocol"); } this.bidi = ((HasBiDi) driver).getBiDi(); this.browsingContextIds = browsingContextIds; } public void onBrowsingContextCreated(Consumer<BrowsingContextInfo> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(browsingContextCreated, consumer); } else { this.bidi.addListener(browsingContextIds, browsingContextCreated, consumer); } } public void onBrowsingContextDestroyed(Consumer<BrowsingContextInfo> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(browsingContextDestroyed, consumer); } else { this.bidi.addListener(browsingContextIds, browsingContextDestroyed, consumer); } } public void onNavigationStarted(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.navigationStarted", consumer); } public void onFragmentNavigated(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.fragmentNavigated", consumer); } public void onDomContentLoaded(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.domContentLoaded", consumer); } public void onBrowsingContextLoaded(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.load", consumer); } public void onDownloadWillBegin(Consumer<DownloadInfo> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(downloadWillBeginEvent, consumer); } else { this.bidi.addListener(browsingContextIds, downloadWillBeginEvent, consumer); } } public void onDownloadEnd(Consumer<DownloadEnded> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(downloadWillEndEvent, consumer); } else { this.bidi.addListener(browsingContextIds, downloadWillEndEvent, consumer); } } public void onNavigationAborted(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.navigationAborted", consumer); } public void onNavigationFailed(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.navigationFailed", consumer); } public void onNavigationCommitted(Consumer<NavigationInfo> consumer) { addNavigationEventListener("browsingContext.navigationCommitted", consumer); } public void onUserPromptClosed(Consumer<UserPromptClosed> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(userPromptClosed, consumer); } else { this.bidi.addListener(browsingContextIds, userPromptClosed, consumer); } } public void onUserPromptOpened(Consumer<UserPromptOpened> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(userPromptOpened, consumer); } else { this.bidi.addListener(browsingContextIds, userPromptOpened, consumer); } } public void onHistoryUpdated(Consumer<HistoryUpdated> consumer) { if (browsingContextIds.isEmpty()) { this.bidi.addListener(historyUpdated, consumer); } else { this.bidi.addListener(browsingContextIds, historyUpdated, consumer); } } private void addNavigationEventListener(String name, Consumer<NavigationInfo> consumer) { Event<NavigationInfo> navigationEvent = new Event<>(name, navigationInfoMapper); navigationEventSet.add(navigationEvent); if (browsingContextIds.isEmpty()) { this.bidi.addListener(navigationEvent, consumer); } else { this.bidi.addListener(browsingContextIds, navigationEvent, consumer); } } @Override public void close() { this.bidi.clearListener(browsingContextCreated); this.bidi.clearListener(browsingContextDestroyed); this.bidi.clearListener(userPromptOpened); this.bidi.clearListener(userPromptClosed); this.bidi.clearListener(historyUpdated); this.bidi.clearListener(downloadWillBeginEvent); this.bidi.clearListener(downloadWillEndEvent);

qodo-merge-pro · 2025-09-24T11:57:03Z

PR Code Suggestions ✨

Latest suggestions up to 8b155ab

Category	Suggestion	Impact
General	Support both filePath key casings In `DownloadCompleted.fromJson`, add support for both `filePath` and `filepath` JSON keys to correctly parse the downloaded file's path, prioritizing the camelCase version as per the specification. java/src/org/openqa/selenium/bidi/browsingcontext/DownloadCompleted.java [73-75] -case "filepath": +case "filePath": filepath = input.read(String.class); break; +case "filepath": + // Fallback for implementations using lowercase + if (filepath == null) { + filepath = input.read(String.class); + } else { + input.skipValue(); + } + break; + Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies that the WebDriver BiDi specification uses `filePath` (camelCase), while the code only handles `filepath`. This is a bug fix that ensures compatibility with spec-compliant browsers, preventing the file path from being missed during JSON parsing.	Medium
General	✅ ~~Fix inconsistent handler naming~~ Suggestion Impact: The commit renames downloadWillEndMapper to downloadEndMapper and downloadWillEndEvent to downloadEndEvent, and updates all their usages including addListener and clearListener calls. code diff: - private final Function<Map<String, Object>, DownloadEnded> downloadWillEndMapper = + private final Function<Map<String, Object>, DownloadEnded> downloadEndMapper = params -> { try (StringReader reader = new StringReader(JSON.toJson(params)); JsonInput input = JSON.newInput(reader)) { @@ -94,8 +94,8 @@ private final Event<DownloadInfo> downloadWillBeginEvent = new Event<>("browsingContext.downloadWillBegin", downloadWillBeginMapper); - private final Event<DownloadEnded> downloadWillEndEvent = - new Event<>("browsingContext.downloadEnd", downloadWillEndMapper); + private final Event<DownloadEnded> downloadEndEvent = + new Event<>("browsingContext.downloadEnd", downloadEndMapper); private final Event<UserPromptOpened> userPromptOpened = new Event<>( @@ -179,9 +179,9 @@ public void onDownloadEnd(Consumer<DownloadEnded> consumer) { if (browsingContextIds.isEmpty()) { - this.bidi.addListener(downloadWillEndEvent, consumer); - } else { - this.bidi.addListener(browsingContextIds, downloadWillEndEvent, consumer); + this.bidi.addListener(downloadEndEvent, consumer); + } else { + this.bidi.addListener(browsingContextIds, downloadEndEvent, consumer); } } @@ -241,7 +241,7 @@ this.bidi.clearListener(userPromptClosed); this.bidi.clearListener(historyUpdated); this.bidi.clearListener(downloadWillBeginEvent); - this.bidi.clearListener(downloadWillEndEvent); + this.bidi.clearListener(downloadEndEvent); navigationEventSet.forEach(this.bidi::clearListener); In `BrowsingContextInspector`, rename the `downloadWillEndMapper` and `downloadWillEndEvent` fields to `downloadEndMapper` and `downloadEndEvent` to match the event name `browsingContext.downloadEnd`, and update their usages accordingly. java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java [68-186] -private final Function<Map<String, Object>, DownloadEnded> downloadWillEndMapper = +private final Function<Map<String, Object>, DownloadEnded> downloadEndMapper = params -> { try (StringReader reader = new StringReader(JSON.toJson(params)); - JsonInput input = JSON.newInput(reader)) { + JsonInput input = JSON.newInput(reader)) { return input.read(DownloadEnded.class); } }; -... - -private final Event<DownloadEnded> downloadWillEndEvent = - new Event<>("browsingContext.downloadEnd", downloadWillEndMapper); - -... +private final Event<DownloadEnded> downloadEndEvent = + new Event<>("browsingContext.downloadEnd", downloadEndMapper); public void onDownloadEnd(Consumer<DownloadEnded> consumer) { if (browsingContextIds.isEmpty()) { - this.bidi.addListener(downloadWillEndEvent, consumer); + this.bidi.addListener(downloadEndEvent, consumer); } else { - this.bidi.addListener(browsingContextIds, downloadWillEndEvent, consumer); + this.bidi.addListener(browsingContextIds, downloadEndEvent, consumer); } } +@Override +public void close() { + this.bidi.clearListener(browsingContextCreated); + this.bidi.clearListener(browsingContextDestroyed); + this.bidi.clearListener(userPromptOpened); + this.bidi.clearListener(userPromptClosed); + this.bidi.clearListener(historyUpdated); + this.bidi.clearListener(downloadWillBeginEvent); + this.bidi.clearListener(downloadEndEvent); + + navigationEventSet.forEach(this.bidi::clearListener); +} + `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 4 __ Why: The suggestion improves code consistency by renaming internal variables like `downloadWillEndMapper` and `downloadWillEndEvent` to align with the event name `browsingContext.downloadEnd`. This enhances readability and maintainability, though it is a minor style improvement.	Low
Possible issue	Validate required fields early In `DownloadEnded.fromJson`, add validation to ensure required fields like `status`, `context`, `timestamp`, and `url` are not null after parsing the JSON, throwing an `IllegalArgumentException` if any are missing. java/src/org/openqa/selenium/bidi/browsingcontext/DownloadEnded.java [30-79] public static DownloadEnded fromJson(JsonInput input) { String browsingContextId = null; String navigationId = null; - long timestamp = 0; + Long timestamp = null; String url = null; String status = null; String filepath = null; input.beginObject(); while (input.hasNext()) { switch (input.nextName()) { case "context": browsingContextId = input.read(String.class); break; case "navigation": navigationId = input.read(String.class); break; case "timestamp": timestamp = input.read(Long.class); break; case "url": url = input.read(String.class); break; case "status": status = input.read(String.class); break; case "filepath": filepath = input.read(String.class); break; default: input.skipValue(); break; } } input.endObject(); - // Create the appropriate object based on status + if (status == null) { + throw new IllegalArgumentException("Missing required field: status"); + } + if (browsingContextId == null) { + throw new IllegalArgumentException("Missing required field: context"); + } + if (timestamp == null) { + throw new IllegalArgumentException("Missing required field: timestamp"); + } + if (url == null) { + throw new IllegalArgumentException("Missing required field: url"); + } + if ("canceled".equals(status)) { DownloadCanceled canceled = new DownloadCanceled(browsingContextId, navigationId, timestamp, url, status); return new DownloadEnded(canceled); } else if ("complete".equals(status)) { DownloadCompleted completed = new DownloadCompleted(browsingContextId, navigationId, timestamp, url, status, filepath); return new DownloadEnded(completed); } else { throw new IllegalArgumentException( "status must be either 'canceled' or 'complete', but got: " + status); } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that missing fields in the JSON payload can lead to partially initialized objects and later runtime errors. Adding explicit checks improves robustness and provides clearer error messages, which is a good practice for error handling.	Medium
Update

Previous suggestions

✅ Suggestions up to commit 173391f

Category	Suggestion	Impact
High-level	Simplify the class hierarchy and deserialization Refactor the class hierarchy by making `DownloadEnded` an abstract base class extended by `DownloadCompleted` and `DownloadCanceled`. This change will remove the need for the wrapper class, eliminate redundant `fromJson` methods in the subclasses, and simplify the overall design. Examples: java/src/org/openqa/selenium/bidi/browsingcontext/DownloadEnded.java [22-93] public class DownloadEnded { private final NavigationInfo downloadParams; public DownloadEnded(NavigationInfo downloadParams) { this.downloadParams = downloadParams; } public static DownloadEnded fromJson(JsonInput input) { String browsingContextId = null; ... (clipped 62 lines) java/src/org/openqa/selenium/bidi/browsingcontext/DownloadCompleted.java [39-86] public static DownloadCompleted fromJson(JsonInput input) { String browsingContextId = null; String navigationId = null; long timestamp = 0; String url = null; String status = "complete"; String filepath = null; input.beginObject(); while (input.hasNext()) { ... (clipped 38 lines) Solution Walkthrough: Before: // In DownloadEnded.java public class DownloadEnded { private final NavigationInfo downloadParams; // ... constructor ... public static DownloadEnded fromJson(JsonInput input) { // ... manually parse all fields ... if ("canceled".equals(status)) { DownloadCanceled canceled = new DownloadCanceled(...); return new DownloadEnded(canceled); } else if ("complete".equals(status)) { DownloadCompleted completed = new DownloadCompleted(...); return new DownloadEnded(completed); } // ... } public NavigationInfo getDownloadParams() { /* ... */ } } After: // In DownloadEnded.java public abstract class DownloadEnded extends NavigationInfo { // ... common fields and methods ... public static DownloadEnded fromJson(JsonInput input) { // ... manually parse all fields ... if ("canceled".equals(status)) { return new DownloadCanceled(...); // Returns subclass instance } else if ("complete".equals(status)) { return new DownloadCompleted(...); // Returns subclass instance } // ... } } // DownloadCompleted and DownloadCanceled would extend DownloadEnded // and their own fromJson methods would be removed. Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies dead code (`fromJson` in subclasses) and proposes a significant design improvement that simplifies the class structure, removes an unnecessary wrapper class, and creates a more direct polymorphic API.	Medium
General	✅ ~~Refactor to remove duplicated JSON parsing~~ Suggestion Impact: The method was refactored to read the JSON into a Map, extract status, and delegate to DownloadCanceled.fromJson or DownloadCompleted.fromJson using a new JsonInput, removing the previous manual field-by-field parsing and object construction. code diff: public static DownloadEnded fromJson(JsonInput input) { - String browsingContextId = null; - String navigationId = null; - long timestamp = 0; - String url = null; - String status = null; - String filepath = null; + Map<String, Object> jsonMap = input.read(Map.class); + String status = (String) jsonMap.get("status"); - input.beginObject(); - while (input.hasNext()) { - switch (input.nextName()) { - case "context": - browsingContextId = input.read(String.class); - break; - case "navigation": - navigationId = input.read(String.class); - break; - case "timestamp": - timestamp = input.read(Long.class); - break; - case "url": - url = input.read(String.class); - break; - case "status": - status = input.read(String.class); - break; - case "filepath": - filepath = input.read(String.class); - break; - default: - input.skipValue(); - break; + try (StringReader reader = new StringReader(new Json().toJson(jsonMap)); + JsonInput jsonInput = new Json().newInput(reader)) { + if ("canceled".equals(status)) { + return new DownloadEnded(DownloadCanceled.fromJson(jsonInput)); + } else if ("complete".equals(status)) { + return new DownloadEnded(DownloadCompleted.fromJson(jsonInput)); + } else { + throw new IllegalArgumentException( + "status must be either 'canceled' or 'complete', but got: " + status); } - } - input.endObject(); - - // Create the appropriate object based on status - if ("canceled".equals(status)) { - DownloadCanceled canceled = - new DownloadCanceled(browsingContextId, navigationId, timestamp, url, status); - return new DownloadEnded(canceled); - } else if ("complete".equals(status)) { - DownloadCompleted completed = - new DownloadCompleted(browsingContextId, navigationId, timestamp, url, status, filepath); - return new DownloadEnded(completed); - } else { - throw new IllegalArgumentException( - "status must be either 'canceled' or 'complete', but got: " + status); } Refactor the `DownloadEnded.fromJson` method to eliminate redundant JSON parsing logic by delegating deserialization to the `DownloadCanceled.fromJson` or `DownloadCompleted.fromJson` methods based on the `status` field. java/src/org/openqa/selenium/bidi/browsingcontext/DownloadEnded.java [30-79] public static DownloadEnded fromJson(JsonInput input) { - String browsingContextId = null; - String navigationId = null; - long timestamp = 0; - String url = null; - String status = null; - String filepath = null; + Map<String, Object> jsonMap = input.read(Map.class); + String status = (String) jsonMap.get("status"); - input.beginObject(); - while (input.hasNext()) { - switch (input.nextName()) { - case "context": - browsingContextId = input.read(String.class); - break; - case "navigation": - navigationId = input.read(String.class); - break; - case "timestamp": - timestamp = input.read(Long.class); - break; - case "url": - url = input.read(String.class); - break; - case "status": - status = input.read(String.class); - break; - case "filepath": - filepath = input.read(String.class); - break; - default: - input.skipValue(); - break; + try (StringReader reader = new StringReader(new Json().toJson(jsonMap)); + JsonInput jsonInput = new Json().newInput(reader)) { + if ("canceled".equals(status)) { + return new DownloadEnded(DownloadCanceled.fromJson(jsonInput)); + } else if ("complete".equals(status)) { + return new DownloadEnded(DownloadCompleted.fromJson(jsonInput)); + } else { + throw new IllegalArgumentException( + "status must be either 'canceled' or 'complete', but got: " + status); } - } - input.endObject(); - - // Create the appropriate object based on status - if ("canceled".equals(status)) { - DownloadCanceled canceled = - new DownloadCanceled(browsingContextId, navigationId, timestamp, url, status); - return new DownloadEnded(canceled); - } else if ("complete".equals(status)) { - DownloadCompleted completed = - new DownloadCompleted(browsingContextId, navigationId, timestamp, url, status, filepath); - return new DownloadEnded(completed); - } else { - throw new IllegalArgumentException( - "status must be either 'canceled' or 'complete', but got: " + status); } } `[Suggestion processed]` Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies duplicated JSON parsing logic in `DownloadEnded.fromJson` and proposes a valid refactoring to delegate deserialization, which improves code structure and maintainability.	Low
Learned best practice	✅ ~~Use accurate event naming~~ Suggestion Impact: The commit renamed downloadWillEndEvent to downloadEndEvent, updated the mapper name to downloadEndMapper, and changed all usages in listener registration and clearing accordingly. code diff: - private final Function<Map<String, Object>, DownloadEnded> downloadWillEndMapper = + private final Function<Map<String, Object>, DownloadEnded> downloadEndMapper = params -> { try (StringReader reader = new StringReader(JSON.toJson(params)); JsonInput input = JSON.newInput(reader)) { @@ -94,8 +94,8 @@ private final Event<DownloadInfo> downloadWillBeginEvent = new Event<>("browsingContext.downloadWillBegin", downloadWillBeginMapper); - private final Event<DownloadEnded> downloadWillEndEvent = - new Event<>("browsingContext.downloadEnd", downloadWillEndMapper); + private final Event<DownloadEnded> downloadEndEvent = + new Event<>("browsingContext.downloadEnd", downloadEndMapper); private final Event<UserPromptOpened> userPromptOpened = new Event<>( @@ -179,9 +179,9 @@ public void onDownloadEnd(Consumer<DownloadEnded> consumer) { if (browsingContextIds.isEmpty()) { - this.bidi.addListener(downloadWillEndEvent, consumer); - } else { - this.bidi.addListener(browsingContextIds, downloadWillEndEvent, consumer); + this.bidi.addListener(downloadEndEvent, consumer); + } else { + this.bidi.addListener(browsingContextIds, downloadEndEvent, consumer); } } @@ -241,7 +241,7 @@ this.bidi.clearListener(userPromptClosed); this.bidi.clearListener(historyUpdated); this.bidi.clearListener(downloadWillBeginEvent); - this.bidi.clearListener(downloadWillEndEvent); + this.bidi.clearListener(downloadEndEvent); navigationEventSet.forEach(this.bidi::clearListener); Rename `downloadWillEndEvent` to reflect the actual event name (`downloadEnd`) for clarity and consistency with other events. This avoids confusion and aligns with event semantics. java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java [97-247] -private final Event<DownloadEnded> downloadWillEndEvent = +private final Event<DownloadEnded> downloadEndEvent = new Event<>("browsingContext.downloadEnd", downloadWillEndMapper); ... public void onDownloadEnd(Consumer<DownloadEnded> consumer) { if (browsingContextIds.isEmpty()) { - this.bidi.addListener(downloadWillEndEvent, consumer); + this.bidi.addListener(downloadEndEvent, consumer); } else { - this.bidi.addListener(browsingContextIds, downloadWillEndEvent, consumer); + this.bidi.addListener(browsingContextIds, downloadEndEvent, consumer); } } ... public void close() { this.bidi.clearListener(browsingContextCreated); this.bidi.clearListener(browsingContextDestroyed); this.bidi.clearListener(userPromptOpened); this.bidi.clearListener(userPromptClosed); this.bidi.clearListener(historyUpdated); this.bidi.clearListener(downloadWillBeginEvent); - this.bidi.clearListener(downloadWillEndEvent); + this.bidi.clearListener(downloadEndEvent); navigationEventSet.forEach(this.bidi::clearListener); } Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Always close or dispose resources and clear listeners to prevent leaks; ensure new listeners are cleared on shutdown using consistent fields.	Low

java/src/org/openqa/selenium/bidi/browsingcontext/DownloadEnded.java

…d.java Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>

asolntsev

suggested minor changes.

java/src/org/openqa/selenium/bidi/browsingcontext/DownloadCanceled.java

java/src/org/openqa/selenium/bidi/browsingcontext/DownloadCompleted.java

java/src/org/openqa/selenium/bidi/browsingcontext/DownloadEnded.java

Delta456 · 2025-09-25T10:01:40Z

Ruby failure is unrelated to my PR.

diemol

Just a minor modification requested.

java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java

diemol

Thank you!

Delta456 · 2025-10-28T10:48:57Z

I have run the CI twice but these tests fail which are unrelated to my changes:

 (10:23:55) FAIL: //dotnet/test/common:DevTools/DevToolsNetworkTest-chrome (see /home/runner/.bazel/execroot/_main/bazel-out/k8-fastbuild-ST-7636bdc63bf0/testlogs/dotnet/test/common/DevTools/DevToolsNetworkTest-chrome/test.log)

FAILED: //dotnet/test/common:DevTools/DevToolsNetworkTest-chrome (Summary)
      /home/runner/.bazel/execroot/_main/bazel-out/k8-fastbuild-ST-7636bdc63bf0/testlogs/dotnet/test/common/DevTools/DevToolsNetworkTest-chrome/test.log
      /home/runner/.bazel/execroot/_main/bazel-out/k8-fastbuild-ST-7636bdc63bf0/testlogs/dotnet/test/common/DevTools/DevToolsNetworkTest-chrome/test_attempts/attempt_1.log
(10:23:55) INFO: From Testing //dotnet/test/common:DevTools/DevToolsNetworkTest-chrome:

diemol · 2025-10-28T10:59:24Z

@Delta456 They are failing because ChromeDriver 142 is not available, I think.

[java][BiDi] implement browsingContext.downloadEnd

173391f

Delta456 requested review from navin772 and pujagani September 24, 2025 11:54

selenium-ci added C-java Java Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Sep 24, 2025

qodo-merge-pro bot added the Review effort 2/5 label Sep 24, 2025

fmt

8b155ab

qodo-merge-pro bot reviewed Sep 24, 2025

View reviewed changes

java/src/org/openqa/selenium/bidi/browsingcontext/DownloadEnded.java Outdated Show resolved Hide resolved

Delta456 and others added 4 commits September 24, 2025 17:57

Update java/src/org/openqa/selenium/bidi/browsingcontext/DownloadEnde…

08b5b6a

…d.java Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>

add imports

f8b4a45

rename

97887a7

revert old test (was accidental)

b8af810

asolntsev added this to the Selenium 4.36 milestone Sep 24, 2025

asolntsev approved these changes Sep 24, 2025

View reviewed changes

Delta456 added 2 commits September 25, 2025 13:05

use consts and util.objects

cca9625

Merge branch 'trunk' into download_end

4696e57

Delta456 requested review from asolntsev and cgoldberg September 25, 2025 07:39

asolntsev approved these changes Sep 25, 2025

View reviewed changes

Delta456 added 6 commits September 27, 2025 23:16

Merge branch 'trunk' into download_end

ffe6504

Merge branch 'trunk' into download_end

de7731d

Merge branch 'trunk' into download_end

98a98b0

Merge branch 'trunk' into download_end

1be7708

Merge branch 'trunk' into download_end

44f6e99

Merge branch 'trunk' into download_end

e7d996f

Delta456 requested a review from diemol October 28, 2025 09:19

diemol requested changes Oct 28, 2025

View reviewed changes

java/src/org/openqa/selenium/bidi/module/BrowsingContextInspector.java Outdated Show resolved Hide resolved

use single imports

785e958

Delta456 requested a review from diemol October 28, 2025 09:38

diemol approved these changes Oct 28, 2025

View reviewed changes

Uh oh!

Uh oh!

[java][BiDi] implement browsingContext.downloadEnd event #16347

Are you sure you want to change the base?

[java][BiDi] implement browsingContext.downloadEnd event #16347

Conversation

Delta456 commented Sep 24, 2025 • edited by qodo-merge-pro bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-merge-pro bot commented Sep 24, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-merge-pro bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

Uh oh!

asolntsev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Delta456 commented Sep 25, 2025 • edited by navin772 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

diemol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

diemol left a comment

Choose a reason for hiding this comment

Uh oh!

Delta456 commented Oct 28, 2025

Uh oh!

diemol commented Oct 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[java][BiDi] implement `browsingContext.downloadEnd` event #16347

[java][BiDi] implement `browsingContext.downloadEnd` event #16347

Delta456 commented Sep 24, 2025 •

edited by qodo-merge-pro bot

Loading

qodo-merge-pro bot commented Sep 24, 2025 •

edited

Loading

Delta456 commented Sep 25, 2025 •

edited by navin772

Loading